Skip to content

Conversation

@fmuenkel
Copy link
Contributor

Overview: What does this pull request change?

More work towards completing #3375.

Motivation and Explanation: Why and how do your changes improve the library?

This PR includes some type annotations to utils/hashing.py to complete typing for cairo_renderer.py. Typing for utils/hashing.py should be completed in another PR.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

self.static_mobjects: list[Mobject] = []
self.time_progression: tqdm[float] | None = None
self.duration: float | None = None
self.duration: float = 0.0
Copy link
Contributor Author

@fmuenkel fmuenkel Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell setting self.duration = 0.0, will not cause any problems, but it avoids having to deal with it being None when calculating number of static frames or self.time.

Iterable[Mobject]
The static image computed.
"""
self.static_image = None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other methods and variables self.static_image should ideally be renamed to self.static_frame, but that can be done in another PR.

@henrikmidtiby henrikmidtiby mentioned this pull request Aug 13, 2025
22 tasks
Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the type annotations, especially those for manim.utils.hashing! That's a pretty difficult module to type properly.

I left some change requests:

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Dev Board Aug 13, 2025
fmuenkel and others added 3 commits August 27, 2025 00:26
Co-authored-by: Francisco Manríquez Novoa <[email protected]>
Co-authored-by: Francisco Manríquez Novoa <[email protected]>
Co-authored-by: Francisco Manríquez Novoa <[email protected]>
@fmuenkel fmuenkel marked this pull request as draft August 26, 2025 23:08
@fmuenkel fmuenkel marked this pull request as ready for review August 27, 2025 09:41
@fmuenkel fmuenkel requested a review from chopan050 August 27, 2025 09:41
Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay.

LGTM! Thanks for your contribution!

@github-project-automation github-project-automation bot moved this from 👀 In review to 👍 To be merged in Dev Board Oct 24, 2025
@chopan050 chopan050 enabled auto-merge (squash) October 24, 2025 19:09
@chopan050 chopan050 merged commit 4dd2937 into ManimCommunity:main Oct 24, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants